new: support annotated tags as merge source #29
Conversation
vaab
force-pushed
the
master
branch
3 times, most recently
from
d6351da to
54c72e6
Compare
2 similar comments
|
As a note: It seems that |
|
@vaab Thank you for your proposal. Back from some days off, I'll review your changes and issues in the coming days.. |
|
I don' know if it's possible to reconsider this PR ? Its results are nice. Thanks |
|
I just tested this PR and it worked also for me. It's a lot of refactoring but the code looks good to me. Regards, lmi |
There was a problem hiding this comment.
I think I've found an issue with this PR.
With such a gitaggregate.yml:
./src/odoo:
remotes:
odoo: https://github.com/odoo/odoo.git
origin: ssh://[email protected]/acsone/odoo.git
merges:
- odoo 13.0
- origin 13.0-with-setup
target: origin 13.0-mybranchgitaggreagate -c gitaggregate.yml -p produces this error:
(INFO) [14:57:13] git_aggregator.repo odoo Push 13.0-mybranch to origin
error: src refspec 13.0-mybranch does not match any
error: failed to push some refs to 'ssh://[email protected]/acsone/odoo.git'
(ERROR) [14:57:13] git_aggregator.repo odoo /home/me/odoo-vestibus/src/odoo> error calling ['git', 'push', '-f', 'origin', '13.0-vst_master']
Traceback (most recent call last):
File "/home/me/git-aggregator/git_aggregator/main.py", line 206, in aggregate_repo
repo.push()
File "/home/me/git-aggregator/git_aggregator/repo.py", line 299, in push
self.log_call(['git', 'push', '-f', remote, branch], cwd=self.cwd)
File "/home/me/git-aggregator/git_aggregator/repo.py", line 177, in log_call
ret = callwith(cmd, **kw)
File "/usr/lib/python3.8/subprocess.py", line 364, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'push', '-f', 'origin', '13.0-mybranch']' returned non-zero exit status 1.
|
Sorry for the delay on my end. And many thanks for all your careful test and review. I am quite busy now, but will try to find some time this afternoon. No promises. I'm aware that it misses tests and all, but I wasn't sure it was a change that you would accept. I think it is more than important that some tests are written to be merged along with this PR. |
…csone#28) Avoids also contacting twice (``fetch`` and then ``pull``) remote before merging. Signed-off-by: Valentin Lab <[email protected]>
|
I've added 2 tests:
And I made the second test pass to fix the issue that @sbidoul brought up. May I bring up the fact that my usage of |
|
If I understand this correctly, the PR affects mainly to the push action, right? We're not using that, so OK for me. |
|
I tested again, works fine now. @vaab can you rebase and suggest a changelog entry? Then I'll cut a release. |
|
@vaab could you rebase and propose a changelog entry that summarizes this change? |
|
Sorry about the delay, I'll be ready to do that starting from the 7th of September. Will that be okay ? |
|
@vaab sure, no worries :) |
Avoids also contacting twice (
fetchand thenpull) remote before merging.This current proposal is only one commit, and has no proper tests yet (although, it passes all existing test). If you are interested in this proposal, I will be more than happy to add proper tests and make changes to it to comply with any of your requirements.
It might also fall short in some of your basic behavior if they are not covered by tests.
This solves #27 and #28